CMP-2615: Add a check aggregate to the compliance scan metadata #588

Vincent056 · 2024-08-13T16:23:39Z

Implement a total check count as an annotation of the ComplianceScan, we will add an annotation compliance.openshift.io/check-count to every compliancescan object when a scan is in Done state.

Noted: This annotation is removed when a new scanPhase begins.

example of output

[vincent@node test-file]$ oc get scan ocp4-cis -o yaml
apiVersion: compliance.openshift.io/v1alpha1
kind: ComplianceScan
metadata:
  annotations:
    compliance.openshift.io/check-count: "89"
  creationTimestamp: "2024-08-15T04:42:36Z"
  finalizers:
  - scan.finalizers.compliance.openshift.io
  generation: 1
  labels:
    compliance.openshift.io/profile-guid: a230315d-3e4a-5b58-b00f-f96f1553e036
    compliance.openshift.io/suite: ocp4-cis-ssb
  name: ocp4-cis

openshift-ci-robot · 2024-08-13T16:23:42Z

@Vincent056: This pull request references CMP-2615 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set.

In response to this:

Implement a total check count as an annotation of the ComplianceScan, we will add an annotation compliance.openshift.io/check-count to every compliancescan object when a scan is in Done state.

Noted: This annotation is removed when a new scanPhase begins.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

github-actions · 2024-08-13T17:59:31Z

🤖 To deploy this PR, run the following command:

make catalog-deploy CATALOG_IMG=ghcr.io/complianceascode/compliance-operator-catalog:588

github-actions · 2024-08-13T23:25:20Z

🤖 To deploy this PR, run the following command:

make catalog-deploy CATALOG_IMG=ghcr.io/complianceascode/compliance-operator-catalog:588

xiaojiey · 2024-08-15T03:25:33Z

@Vincent056 I cannot see any annotation in the compliancescan. Could you please take a look? Thanks.

% oc get sub                             
NAME                      PACKAGE               SOURCE                CHANNEL
compliance-operator-sub   compliance-operator   compliance-operator   alpha
% oc get catalogsource compliance-operator -n openshift-marketplace -o=jsonpath={.spec.image}
ghcr.io/complianceascode/compliance-operator-catalog:588%                                                                                                                                % cat ssb.yaml 
apiVersion: compliance.openshift.io/v1alpha1
kind: ScanSettingBinding
metadata:
  name: stig-compliance
  namespace: openshift-compliance
profiles:
  - name: ocp4-stig-node
    kind: Profile
    apiGroup: compliance.openshift.io/v1alpha1
  - name: ocp4-stig
    kind: Profile
    apiGroup: compliance.openshift.io/v1alpha1
  - name: rhcos4-stig
    kind: Profile
    apiGroup: compliance.openshift.io/v1alpha1
settingsRef:
  name: default
  kind: ScanSetting
  apiGroup: compliance.openshift.io/v1alpha1
% oc get scan
NAME                    PHASE   RESULT
ocp4-stig               DONE    NON-COMPLIANT
ocp4-stig-node-master   DONE    NON-COMPLIANT
ocp4-stig-node-worker   DONE    NON-COMPLIANT
rhcos4-stig-master      DONE    NON-COMPLIANT
rhcos4-stig-worker      DONE    NON-COMPLIANT

% oc get scan -o yaml | grep -i anno 
% oc get scan ocp4-stig -o=jsonpath={.metadata.annotations}
% oc get scan rhcos4-stig-master -o=jsonpath={.metadata.annotations}
% oc get scan ocp4-stig -o=jsonpath={.metadata} | jq -r    
{
  "creationTimestamp": "2024-08-15T02:56:36Z",
  "finalizers": [
    "scan.finalizers.compliance.openshift.io"
  ],
  "generation": 1,
  "labels": {
    "compliance.openshift.io/profile-guid": "2cbba61f-f813-5468-b439-3b01e26f757b",
    "compliance.openshift.io/suite": "stig-compliance"
  },
  "managedFields": [
    {
      "apiVersion": "compliance.openshift.io/v1alpha1",
      "fieldsType": "FieldsV1",
      "fieldsV1": {
        "f:metadata": {
          "f:finalizers": {
            ".": {},
            "v:\"scan.finalizers.compliance.openshift.io\"": {}
          },
          "f:labels": {
            ".": {},
            "f:compliance.openshift.io/profile-guid": {},
            "f:compliance.openshift.io/suite": {}
          },
          "f:ownerReferences": {
            ".": {},
            "k:{\"uid\":\"3b9a95cb-492e-4478-be32-1dc1d05eea60\"}": {}
          }
        },
        "f:spec": {
          ".": {},
          "f:content": {},
          "f:contentImage": {},
          "f:maxRetryOnTimeout": {},
          "f:profile": {},
          "f:rawResultStorage": {
            ".": {},
            "f:nodeSelector": {
              ".": {},
              "f:node-role.kubernetes.io/master": {}
            },
            "f:pvAccessModes": {},
            "f:rotation": {},
            "f:size": {},
            "f:tolerations": {}
          },
          "f:scanTolerations": {},
          "f:scanType": {},
          "f:showNotApplicable": {},
          "f:strictNodeScan": {},
          "f:timeout": {}
        }
      },
      "manager": "compliance-operator",
      "operation": "Update",
      "time": "2024-08-15T02:56:37Z"
    },
    {
      "apiVersion": "compliance.openshift.io/v1alpha1",
      "fieldsType": "FieldsV1",
      "fieldsV1": {
        "f:status": {
          ".": {},
          "f:conditions": {},
          "f:endTimestamp": {},
          "f:phase": {},
          "f:remainingRetries": {},
          "f:result": {},
          "f:resultsStorage": {
            ".": {},
            "f:name": {},
            "f:namespace": {}
          },
          "f:startTimestamp": {},
          "f:warnings": {}
        }
      },
      "manager": "compliance-operator",
      "operation": "Update",
      "subresource": "status",
      "time": "2024-08-15T02:57:37Z"
    }
  ],
  "name": "ocp4-stig",
  "namespace": "openshift-compliance",
  "ownerReferences": [
    {
      "apiVersion": "compliance.openshift.io/v1alpha1",
      "blockOwnerDeletion": true,
      "controller": true,
      "kind": "ComplianceSuite",
      "name": "stig-compliance",
      "uid": "3b9a95cb-492e-4478-be32-1dc1d05eea60"
    }
  ],
  "resourceVersion": "75252",
  "uid": "17b82e87-7cf7-41d0-a77c-1fc54664aeb0"
}

xiaojiey · 2024-08-15T03:25:46Z

/hold for test

github-actions · 2024-08-15T07:57:27Z

🤖 To deploy this PR, run the following command:

make catalog-deploy CATALOG_IMG=ghcr.io/complianceascode/compliance-operator-catalog:588-cc9a0d89b221a015818a2ddd026caf49e9220fc0

xiaojiey · 2024-08-15T12:58:31Z

Verification pass with CATALOG_IMG ghcr.io/complianceascode/compliance-operator-catalog:588-cc9a0d89b221a015818a2ddd026caf49e9220fc0:

% cat ssb.yaml 
apiVersion: compliance.openshift.io/v1alpha1
kind: ScanSettingBinding
metadata:
  name: stig-compliance
  namespace: openshift-compliance
profiles:
  - name: ocp4-stig-node
    kind: Profile
    apiGroup: compliance.openshift.io/v1alpha1
  - name: ocp4-stig
    kind: Profile
    apiGroup: compliance.openshift.io/v1alpha1
  - name: rhcos4-stig
    kind: Profile
    apiGroup: compliance.openshift.io/v1alpha1
settingsRef:
  name: default
  kind: ScanSetting
  apiGroup: compliance.openshift.io/v1alpha1
% oc apply -f ssb.yaml 
scansettingbinding.compliance.openshift.io/stig-compliance created
 % oc get scan
NAME                    PHASE         RESULT
ocp4-stig               DONE          NON-COMPLIANT
ocp4-stig-node-master   DONE          NON-COMPLIANT
ocp4-stig-node-worker   DONE          NON-COMPLIANT
rhcos4-stig-master      AGGREGATING   NOT-AVAILABLE
rhcos4-stig-worker      DONE          NON-COMPLIANT

% oc get ccr -l compliance.openshift.io/scan-name=ocp4-stig --no-headers | wc -l
     113
%  oc get scan ocp4-stig -o=jsonpath={.metadata.annotations} | jq -r
{
  "compliance.openshift.io/check-count": "113"
}
% oc get ccr -l compliance.openshift.io/scan-name=ocp4-stig-node-master --no-headers | wc -l
     100
%  oc get scan ocp4-stig-node-master -o=jsonpath={.metadata.annotations} | jq -r        
{
  "compliance.openshift.io/check-count": "100"
}
% oc get ccr -l compliance.openshift.io/scan-name=rhcos4-stig-master --no-headers | wc -l
     118
%  oc get scan rhcos4-stig-master -o=jsonpath={.metadata.annotations} | jq -r    
{
  "compliance.openshift.io/check-count": "118"
}

pkg/controller/compliancescan/compliancescan_controller.go

rhmdnd · 2024-08-20T19:53:11Z

tests/e2e/serial/main_test.go

@@ -245,6 +245,11 @@ func TestSuiteScan(t *testing.T) {
 		f.AssertCheckRemediation(checkVsyscall.Name, checkVsyscall.Namespace, true)
 	}

+	// ensure scan has total check counts annotation
+	err = f.AssertScanHasTotalCheckCounts(f.OperatorNamespace, workerScanName)


We could assert each scan in the suite has this, too.

Somewhat related, it would be nice to have an E2E assertion (or set of assertions) that check the ComplianceScan object that we could use consistently across the suite. If this test is ever refactored, and this assertion goes missing, our test coverage for the check-count annotation goes with it.

somthing like all scans in the suite has that?

Yeah - it could be a part of the assertion to check for a successful or done scan.

rhmdnd · 2024-08-20T19:56:48Z

pkg/controller/compliancescan/compliancescan_controller.go

@@ -727,6 +760,7 @@ func (r *ReconcileComplianceScan) phaseDoneHandler(h scanTypeHandler, instance *
 			// reset phase
 			logger.Info("Resetting scan")
 			instanceCopy := instance.DeepCopy()
+			delete(instanceCopy.Annotations, compv1alpha1.ComplianceCheckCountAnnotation)


We could add a check in our e2e test utilities that asserts this annotation is deleted while the suite is waiting for the scan to cycle through it's phases. That way we're protecting this from regression if it gets accidentally removed.

@rhmdnd You mean the user manually removing the check count annotation?

Specifically, if line 763 is removed or refactored in a way that doesn't delete the annotation, we'd know about it because we have code in the assertions to make sure it's deleted up until the scan goes into a specific phase.

I wasn't able to get this to work locally.

I moved the logic to a difference place and got it to work.

$ git d diff --git a/pkg/controller/compliancescan/compliancescan_controller.go b/pkg/controller/compliancescan/compliancescan_controller.go index 507f6bbe..d746cafe 100644 --- a/pkg/controller/compliancescan/compliancescan_controller.go +++ b/pkg/controller/compliancescan/compliancescan_controller.go @@ -288,6 +288,7 @@ func (r *ReconcileComplianceScan) phasePendingHandler(instance *compv1alpha1.Com if instance.NeedsRescan() { instanceCopy := instance.DeepCopy() delete(instanceCopy.Annotations, compv1alpha1.ComplianceScanRescanAnnotation) + delete(instanceCopy.Annotations, compv1alpha1.ComplianceCheckCountAnnotation) delete(instanceCopy.Annotations, compv1alpha1.ComplianceScanTimeoutAnnotation) err := r.Client.Update(context.TODO(), instanceCopy) return reconcile.Result{}, err

github-actions · 2024-08-27T14:35:12Z

🤖 To deploy this PR, run the following command:

make catalog-deploy CATALOG_IMG=ghcr.io/complianceascode/compliance-operator-catalog:588-364ee0d7f1963e87ac7805b038eea2e1df84f7e3

Vincent056 · 2024-08-28T16:17:27Z

/retest

rhmdnd · 2024-08-28T20:56:20Z

tests/e2e/serial/main_test.go

+		},
+	}
+
+	err := f.Client.Create(context.TODO(), exampleComplianceSuite, nil)


What's the rationale behind creating a ComplianceSuite directly as opposed to using a ScanSettingBinding?

rhmdnd · 2024-08-28T20:57:49Z

tests/e2e/serial/main_test.go

+		t.Fatal(err)
+	}
+
+	// At this point, both scans should be non-compliant given our current content


nit: These two assertions aren't hurting anything, but they're probably not overly useful for this particular test since we really just care about asserting the check counts for each scan.

Nit: Agree with Lance.. And actually suggest removing these assertions for NonCompliant scans.
When/if rhcos4 moderate profile starts passing out of the box this test will start failing, XD.

rhmdnd

/lgtm

Only minor comments and questions that we can choose to clean up in a follow up.

rhmdnd · 2024-08-28T21:02:41Z

Removing hold due to #588 (comment)

yuumasato · 2024-08-29T08:27:39Z

tests/e2e/serial/main_test.go

+		t.Fatal(err)
+	}
+
+	// At this point, both scans should be non-compliant given our current content


Nit: Agree with Lance.. And actually suggest removing these assertions for NonCompliant scans.
When/if rhcos4 moderate profile starts passing out of the box this test will start failing, XD.

yuumasato · 2024-08-29T08:40:50Z

pkg/controller/compliancescan/compliancescan_controller.go

@@ -727,6 +760,7 @@ func (r *ReconcileComplianceScan) phaseDoneHandler(h scanTypeHandler, instance *
 			// reset phase
 			logger.Info("Resetting scan")
 			instanceCopy := instance.DeepCopy()
+			delete(instanceCopy.Annotations, compv1alpha1.ComplianceCheckCountAnnotation)


@rhmdnd You mean the user manually removing the check count annotation?

github-actions · 2024-08-30T18:55:39Z

🤖 To deploy this PR, run the following command:

make catalog-deploy CATALOG_IMG=ghcr.io/complianceascode/compliance-operator-catalog:588-96718b9cf97b7e7a7e30375beedcab31ec179795

Implement a total check count as an annotation of the ComplianceScan, we will add an annotation compliance.openshift.io/check-count to every compliancescan object when a scan is in done state. Noted: This annotation is removed when a new scanPhase begin.

github-actions · 2024-08-30T21:20:37Z

🤖 To deploy this PR, run the following command:

make catalog-deploy CATALOG_IMG=ghcr.io/complianceascode/compliance-operator-catalog:588-ec93a11d1a140edb0b328f3fef36cff6bc4fd563

github-actions · 2024-08-30T21:21:37Z

🤖 To deploy this PR, run the following command:

make catalog-deploy CATALOG_IMG=ghcr.io/complianceascode/compliance-operator-catalog:588-387b50426002df4a33a99ede43d01c1971663f8f

GroceryBoyJr · 2024-08-30T21:34:51Z

/label docs-approved

GroceryBoyJr · 2024-08-30T21:36:58Z

/assign GroceryBoyJr

rhmdnd

/lgtm

openshift-ci · 2024-08-30T22:03:24Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhmdnd, Vincent056, yuumasato

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [Vincent056,rhmdnd,yuumasato]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

rhmdnd · 2024-08-31T00:06:16Z

/test e2e-aws-parallel

Transient issues with metrics tests :(

#302

Vincent056 · 2024-09-03T14:36:33Z

/test e2e-aws-parallel

rhmdnd · 2024-09-03T20:54:17Z

/test e2e-aws-serial

Failed on a network blip during clean up, but the tests passed fine. Tracking the e2e improvement in #614

openshift-ci-robot added the jira/valid-reference label Aug 13, 2024

openshift-ci bot requested review from rhmdnd and yuumasato August 13, 2024 16:23

openshift-ci bot added the approved label Aug 13, 2024

Vincent056 force-pushed the total_checks branch from b312849 to 8aafdac Compare August 13, 2024 21:38

Vincent056 added this to the 1.6.0 milestone Aug 13, 2024

openshift-ci bot added the do-not-merge/hold label Aug 15, 2024

Vincent056 force-pushed the total_checks branch from 8aafdac to cc9a0d8 Compare August 15, 2024 07:50

rhmdnd reviewed Aug 20, 2024

View reviewed changes

pkg/controller/compliancescan/compliancescan_controller.go Show resolved Hide resolved

rhmdnd reviewed Aug 20, 2024

View reviewed changes

pkg/controller/compliancescan/compliancescan_controller.go Show resolved Hide resolved

rhmdnd reviewed Aug 20, 2024

View reviewed changes

pkg/controller/compliancescan/compliancescan_controller.go Show resolved Hide resolved

rhmdnd reviewed Aug 20, 2024

View reviewed changes

Vincent056 force-pushed the total_checks branch from cc9a0d8 to 364ee0d Compare August 27, 2024 14:27

rhmdnd reviewed Aug 28, 2024

View reviewed changes

rhmdnd approved these changes Aug 28, 2024

View reviewed changes

openshift-ci bot assigned rhmdnd Aug 28, 2024

openshift-ci bot added the lgtm label Aug 28, 2024

rhmdnd removed the do-not-merge/hold label Aug 28, 2024

yuumasato approved these changes Aug 29, 2024

View reviewed changes

openshift-ci bot assigned yuumasato Aug 29, 2024

Vincent056 force-pushed the total_checks branch from 364ee0d to 96718b9 Compare August 30, 2024 18:49

openshift-ci bot removed the lgtm label Aug 30, 2024

Vincent056 force-pushed the total_checks branch from 96718b9 to ec93a11 Compare August 30, 2024 21:13

Vincent056 force-pushed the total_checks branch from ec93a11 to 387b504 Compare August 30, 2024 21:14

openshift-ci bot added the docs-approved label Aug 30, 2024

openshift-ci bot assigned GroceryBoyJr Aug 30, 2024

rhmdnd approved these changes Aug 30, 2024

View reviewed changes

openshift-ci bot added the lgtm label Aug 30, 2024

openshift-merge-bot bot merged commit 067ed66 into master Sep 3, 2024
16 checks passed

openshift-merge-bot bot deleted the total_checks branch September 3, 2024 22:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CMP-2615: Add a check aggregate to the compliance scan metadata #588

CMP-2615: Add a check aggregate to the compliance scan metadata #588

Vincent056 commented Aug 13, 2024 •

edited

Loading

openshift-ci-robot commented Aug 13, 2024

github-actions bot commented Aug 13, 2024

github-actions bot commented Aug 13, 2024

xiaojiey commented Aug 15, 2024

xiaojiey commented Aug 15, 2024

github-actions bot commented Aug 15, 2024

xiaojiey commented Aug 15, 2024

rhmdnd Aug 20, 2024

Vincent056 Aug 27, 2024

rhmdnd Aug 27, 2024

rhmdnd Aug 20, 2024

yuumasato Aug 29, 2024

rhmdnd Aug 29, 2024

rhmdnd Aug 30, 2024

github-actions bot commented Aug 27, 2024

Vincent056 commented Aug 28, 2024

rhmdnd Aug 28, 2024

rhmdnd Aug 28, 2024 •

edited

Loading

yuumasato Aug 29, 2024

rhmdnd left a comment

rhmdnd commented Aug 28, 2024

yuumasato Aug 29, 2024

yuumasato Aug 29, 2024

github-actions bot commented Aug 30, 2024

github-actions bot commented Aug 30, 2024

github-actions bot commented Aug 30, 2024

GroceryBoyJr commented Aug 30, 2024

GroceryBoyJr commented Aug 30, 2024

rhmdnd left a comment

openshift-ci bot commented Aug 30, 2024

rhmdnd commented Aug 31, 2024

Vincent056 commented Sep 3, 2024

rhmdnd commented Sep 3, 2024

CMP-2615: Add a check aggregate to the compliance scan metadata #588

CMP-2615: Add a check aggregate to the compliance scan metadata #588

Conversation

Vincent056 commented Aug 13, 2024 • edited Loading

openshift-ci-robot commented Aug 13, 2024

github-actions bot commented Aug 13, 2024

github-actions bot commented Aug 13, 2024

xiaojiey commented Aug 15, 2024

xiaojiey commented Aug 15, 2024

github-actions bot commented Aug 15, 2024

xiaojiey commented Aug 15, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Aug 27, 2024

Vincent056 commented Aug 28, 2024

Choose a reason for hiding this comment

rhmdnd Aug 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rhmdnd left a comment

Choose a reason for hiding this comment

rhmdnd commented Aug 28, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Aug 30, 2024

github-actions bot commented Aug 30, 2024

github-actions bot commented Aug 30, 2024

GroceryBoyJr commented Aug 30, 2024

GroceryBoyJr commented Aug 30, 2024

rhmdnd left a comment

Choose a reason for hiding this comment

openshift-ci bot commented Aug 30, 2024

rhmdnd commented Aug 31, 2024

Vincent056 commented Sep 3, 2024

rhmdnd commented Sep 3, 2024

Vincent056 commented Aug 13, 2024 •

edited

Loading

rhmdnd Aug 28, 2024 •

edited

Loading